feat: teach HTTP verbs and GET caching on the gallery server-actions card - #1152
Conversation
|
Design rationale: why a second counter, and why this extends the card instead of adding a route Two calls I want on the record. One card, not a new route. Verbs are a property of a server action, not a feature of their own, and the server-actions card already explains the sibling config export nobody thinks of as separate either ( The read reports how many times it actually ran. My first version had the GET return just the domain counter and a server timestamp, and the copy told the visitor to watch the value stay frozen inside the cache window. I drove it in a real browser and the demo does not actually prove anything: two clicks land in the same second, so a cached answer and a fast fresh one look identical. So the payload now carries Verified against a generated app, in a browser: And through the handler, the wire matches what the card claims: One thing I checked and left alone: a POST to the GET action returns 200, not the 405 you might expect. That is the documented over-4KB args fallback doing its job, so the card does not claim a 405. |
vivek7405
left a comment
There was a problem hiding this comment.
Went through this against the real action implementation and drove the card in a browser. The behaviour holds up: the cached read, the tag invalidation, and the headers all do what the copy says. Four things to fix before this is teaching material.
The two that matter most are that the new mutation diverges from the ActionResult envelope every other gallery mutation returns, and that the card credits SSR seeding to the GET verb when seeding is verb-independent. Both are the kind of thing an agent will copy straight out of the scaffold, which is the whole point of the gallery.
The browser check that is red is packages/core/test/routing/browser/page-action-submit.test.js on Firefox, unrelated to this diff (no core change here), but it gates the merge until it is re-run.
vivek7405
left a comment
There was a problem hiding this comment.
Second pass, zoomed out to the surfaces around the demo rather than the demo itself. The interesting result is that fixing the seeding claim on the card left three other places saying the old thing, so this round was mostly about not shipping a scaffold that contradicts itself.
The api template comment and AGENTS.md are fixed here. The same wrong framing is also in the published post blog/get-server-actions-caching.md under What a GET buys, which I left alone and filed as #1153, since editing published marketing prose belongs in its own change with the blog voice rules applied.
I also checked the new test the way I would want someone to check mine: whether rm -rf on the temp dir could follow the node_modules symlink into the repo. It cannot, fs.rm unlinks the symlink itself. The test is clean under Node and Bun and alongside the other scaffold tests, and gallery:clear still strips every new file with a typecheck-clean app after.
vivek7405
left a comment
There was a problem hiding this comment.
Third pass, this one adversarial against the demo rather than the copy, and it was the most productive. The demo had a real re-entrancy bug: ?disabled only lands on the next render commit, so three clicks in one task fired three concurrent reads and prepended three duplicate rows. Confirmed in Chromium before and after, it is one request now.
The other keeper is subtler. The client column labelled "clicked" was sampled after the await, so it was actually the response time, on a card whose entire lesson is contrasting that column against the frozen server value.
I also tightened the test's honesty. It cannot observe a cache at all (no browser, and a GET action is never cached server-side), so the assertion messages no longer imply it proves a bypass, and max-age is pinned to the ten seconds the copy commits to rather than any number.
vivek7405
left a comment
There was a problem hiding this comment.
Fourth pass, checking the previous rounds' fixes rather than the original diff, and one of my own fixes turned out to be wrong. I had rewritten the auth counter-example to say a GET action "is a cacheable read". It is not: a GET with no cache export is no-store, which the gallery itself proves twice (the todo and async-render reads are both GETs with no cache). Caching is opted into by cache, which is what the new card teaches, so the counter-example was arguing from the wrong premise.
Same class of error twice more. The card said any action invoked during SSR is seeded, but seeding needs a fully buffered render, and the gallery's own async-render card streams, so that page emits no seed block at all. And swr was mentioned with no indication it is a key of the cache object rather than a sibling export, which matters because export const swr = 30 is silently ignored: not a check rule, not an error, just no effect.
The api route.ts finding is worth flagging on its own. It calls its actions in-process, so none of the config exports apply there, while the comment above them promised browser-cache behaviour. That template now says so.
One of the reworded comments also broke an existing assertion in scaffold-integration.test.js that the auth query explains itself as per-session. I put the wording back rather than loosening the test. PR body rewritten to cover the whole diff.
vivek7405
left a comment
There was a problem hiding this comment.
Fifth pass, and the headline is that my own correction from last round was wrong. I told an agent to swap in the namespace form of route() to have an endpoint honour the config exports. It reads middleware and validate and nothing else; method, cache, tags, and invalidates are applied by the RPC endpoint alone. AGENTS.md had this exactly right and I contradicted it while fixing something else.
Worth naming the pattern, since it has now happened twice on this PR: every wrong claim here was a plausible generalization written from memory, and every one survived until someone opened the source. The corrections are all read off action-route.js, action-config.js, and actions.js this time.
The rest is the same cleanup reaching further out. The auth counter-example was arguing from a rule this gallery breaks twice, the todo query claimed a hydration re-fetch that its own wiring cannot produce (the page awaits it server-side and passes rows down as a property), and the docs-site data-fetching page still said a GET is served with Cache-Control full stop.
vivek7405
left a comment
There was a problem hiding this comment.
Sixth pass. Nothing in the diff is factually wrong any more, which is the first time that has been true on this PR. What is left is reach: the same two claims this change exists to kill were still alive in five more places, including AGENTS.md itself, which I had edited without fixing the caching half of the same sentence.
One real defect in the demo, found by asking what happens on a deploy rather than on localhost. The card asks you to compare a server timestamp against a browser one, and the action was formatting its side in the SERVER's timezone, so on webjs.dev they would sit hours apart with no explanation. The action returns an ISO instant now and the component formats both.
The results region also had no live-region wiring, on a card whose entire payload is a number you are supposed to watch not move. It now announces, and a failed call is an alert rather than silence.
vivek7405
left a comment
There was a problem hiding this comment.
Seventh pass, clean. Re-verified the last round's fixes against the source rather than trusting them: the ETag is unconditional while Cache-Control and X-Webjs-Tags are gated on cache, eviction is gated on the action having run, route() reads only middleware and validate, and the seed block needs an empty pending list. All four match what the diff now says.
Drove a generated app in Chromium on this head: two reads inside the window both report the same served count with the second never reaching the server, a bump then read forces the bypass, and the two timestamp columns render in the same timezone. gallery:clear strips all 42 paths and leaves a check-clean, typecheck-clean app.
Good to merge from my side.
The full-stack gallery used `method = 'GET'` incidentally in two query files and never showed `cache`, `tags`, or `invalidates` at all, so a scaffolded app only learned the verb surface by accident. The api template already taught it, which left the template most agents generate as the one missing it. Extend the existing server-actions card rather than adding a route: verbs are a property of an action, not a separate feature, and the card already explains the other config export (`middleware`).
The reserved sibling exports are config, not module exports, so
gallery-coverage.json cannot gate them. This guards the case that let the
gap exist: `method`, `cache`, `tags`, and `invalidates` silently absent
from the scaffold while every other surface stayed green.
Also asserts the one-function-per-configured-file rule and that no
gallery action ships `{ public: true }` in code an agent would paste,
since the card teaches that flag as a hazard.
Three fixes from review. The card credited SSR seeding to the GET verb, but seeding facades every 'use server' export whatever its method, so an agent could conclude a read must be a GET to skip the hydration round-trip. The new mutation returned a bare object while every other gallery mutation returns the ActionResult envelope, which would have taught two contracts on the one card that explains actions. Neither handler caught a rejected call, so a failed RPC left a silently inert UI next to a greeter that models the opposite. The 304 wording is also narrowed: this read carries a per-execution counter, so its ETag differs every run and it never answers 304. A read whose result is stable does, which is what the copy now says. Adds a behaviour test that boots a generated app and asserts the wire the card describes, since source assertions alone cannot tell whether the framework honours the config exports.
The api template and AGENTS.md both listed seed-reading among the things `method = 'GET'` buys, which the gallery card now contradicts in print. Seeding facades every 'use server' export whatever its verb, so the GET enumeration is the wrong home for it. AGENTS.md already describes the mechanism correctly under async render. Also corrects the api template's claim that `invalidates` evicts "on success": eviction is gated on the action running without throwing, so a returned failure envelope still evicts.
Three defects in the demo itself. `busy` was set without a re-entrancy check, so `?disabled` (which only lands on the next render commit) let three clicks in one task fire three concurrent reads and prepend three duplicate rows; the signal is now the guard, matching the rate-limit probe. The client column rendered as "clicked" was sampled after the await, so it was the response time on a card whose whole point is contrasting that against the server value. And the shared counters never said they are per-process, which matters on the deployed gallery where another visitor's bump moves your reading. Two other gallery queries still credited SSR seeding to the GET verb, one directory from the card that now denies it. The behaviour test stops claiming coverage its seam cannot provide (no browser, and no server-side cache of a GET action, so nothing there can prove a bypass), and pins max-age to the ten seconds the copy promises.
Review found the card and its neighbours crediting the GET verb with things `cache` actually does. A GET with no `cache` export is `no-store`, and `X-Webjs-Tags` rides only a cached response, so "a GET is a cacheable read" was wrong in the auth counter-example and in the api template. Two more overstatements corrected. Seeding needs a fully buffered render, so a streamed page emits no seed block at all, which the gallery's own async-render card demonstrates. And `swr` is a key of the cache object, never a sibling export, so the copy now shows the object form rather than leaving an agent to invent `export const swr`. The api route.ts calls its actions in-process, which skips every config export. The template now says so instead of promising cache behaviour its own wiring cannot produce.
The previous commit's own correction over-claimed. route() reads only `middleware` and `validate` off a module namespace; method, cache, tags, and invalidates are applied by the RPC endpoint and nowhere else, so telling an agent to swap in the adapter to "honour them" was wrong for four of the six. AGENTS.md had it right all along. Also stops the auth counter-example arguing from a rule the gallery breaks twice (two GET reads ship with no cache window), drops a seeding claim from the todo query that its own wiring never exercises, since the page awaits it server-side and passes rows down as a property, and brings the docs-site data-fetching page in line with the rest.
The demo sat a server-formatted local time next to a browser one, so on any deploy where the two timezones differ the columns it asks you to compare are hours apart. The action now returns an ISO instant and the component formats both, in the visitor's timezone. The results region gains role="status" so a screen-reader user hears the number the card is entirely about, and a failed call is an alert rather than silence. The rest is the same two claims, hunted down wherever they still lived: AGENTS.md still granted Cache-Control and X-Webjs-Tags to the verb, the todo example still credited its first paint to seeding, the blog's get-post still promised a route() boundary that applies the cache exports (and that the blog does not even use), delete-post still said eviction happens on success while its own 401 path proves otherwise, and the docs site described seeding as suppressed per region when the gate is page-wide.
07d7942 to
56ad9d9
Compare
Closes #1151
The full-stack scaffold gallery never taught HTTP-verb server actions.
method = 'GET'appeared only as incidental usage in two query files, and
cache/tags/invalidatesdid not appear as working code anywhere in the UI gallery. The apitemplate already taught the full set, so the template most agents actually generate was
the one missing it.
This extends the EXISTING server-actions card instead of adding a
/features/http-verbsroute. Verbs are a property of a server action, not a separate feature, and that card
already explains the other reserved config export (
middleware), so a second sectionkeeps one concept on one route and does not grow the prune surface.
The demo
modules/server-actions/queries/read-clock.server.ts: a GET action withmethod+cache = 10+tags, commented export by export.modules/server-actions/actions/bump-clock.server.ts: the paired POST mutationdeclaring
invalidatesfor the same tag, returning theActionResultenvelope.modules/server-actions/utils/clock.server.ts: the server-only state both share.modules/server-actions/components/clock-reader.ts: click-driven Read and Bump. Thepayload carries a counter incremented inside the action body, so a browser-cache hit
is visible as a number that does not move, rather than something to squint at.
previously dangled on "or a GET action's
export const cache" with nothing behind it.The corrections that came with it
Writing the card surfaced four claims the scaffold was making that are not true, all of
which would have been copied by an agent reading it:
'use server'export whateverthe verb, and it needs a fully buffered render (a streamed page emits no seed block).
Corrected in
AGENTS.md, the api template, and two other gallery queries.cache, not granted bymethod. A GET with nocacheexport is
no-store. The auth card's counter-example and the api template both saidotherwise.
invalidatesevicts once the action completes without throwing, not "on success",so a returned
{ success: false }envelope still evicts. Fixed in the api template andthe skill reference.
swris a key of the cache object, never a sibling export.export const swrissilently ignored, so the copy shows the object form.
The api
route.tscalls its actions in-process, which skips every config export, so itnow says so instead of promising cache behaviour its own wiring cannot produce.
One further instance of the seeding misattribution is in the published post
blog/get-server-actions-caching.md. Left alone here and filed as #1153, since editingpublished prose belongs in its own change with the blog voice rules applied.
Tests
test/scaffolds/scaffold-gallery.test.js: asserts the card demonstrates every reservedconfig export, imports its component module, keeps one function per configured file, and
ships no
{ public: true }in code. These exports are config, not module exports, sogallery-coverage.jsoncannot gate them.test/scaffolds/scaffold-gallery-actions.test.js(new): boots a generated app andasserts the wire,
private, max-age=10,x-webjs-tags: clock, an ETag, andx-webjs-invalidate: clockon the mutation.Test plan
test/scaffolds/*.test.js, 50/50export const cachereds the new assertion, restoring it greenswebjs checkclean,webjs typecheckcleanwebjs checkcleanservedcount, a bump makes the next read fresh, three clicks in one task fire onerequest, no console errors
gallery:clearstill strips every new file, leaving a typecheck-clean app